-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat/btpm new deployment scripts #72
Conversation
const entryPointAddress = | ||
process.env.ENTRY_POINT_ADDRESS || | ||
"0x5FF137D4b0FDCD49DcA30c7CF57E578a026d2789"; | ||
const verifyingSigner = process.env.PAYMASTER_SIGNER_ADDRESS_PROD || ""; | ||
const DEPLOYER_CONTRACT_ADDRESS = | ||
process.env.DEPLOYER_CONTRACT_ADDRESS_PROD || ""; | ||
|
||
function delay(ms: number) { | ||
return new Promise<void>((resolve) => { | ||
setTimeout(() => { | ||
resolve(); | ||
}, ms); | ||
}); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be managed in a central config.ts
and not duplicated across scripts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DEPLOYER_CONTRACT_ADDRESS and entryPointAddress are applicable to more scripts. Have left them as it is to have control on individual script.
moved delay to utils/
const BiconomyTokenPaymaster = await ethers.getContractFactory( | ||
"BiconomyTokenPaymaster" | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use Typechain
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
async function deployTokenPaymasterContract( | ||
deployerInstance: Deployer, | ||
earlyOwnerAddress: string | ||
): Promise<string | undefined> { | ||
try { | ||
const salt = ethers.utils.keccak256( | ||
ethers.utils.toUtf8Bytes(DEPLOYMENT_SALTS.TOKEN_PAYMASTER_V2) | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be better to create a common deployGeneric
similar to https://github.com/bcnmy/scw-contracts/blob/v2-deployments-with-config/scripts/deploy.ts#L67 and use it across all scripts instead of duplicating deploy -> verify logic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added deployGeneric() in this script and can re use for other contracts in same script (although there aren't any now)
|
||
async function main() { | ||
const accounts = await ethers.getSigners(); | ||
const earlyOwner = await accounts[0].getAddress(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should verify that this is not null and is a valid address
https://docs.ethers.org/v5/api/utils/address/#utils-isAddress
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Summary
Related Issue: # (issue number)
Change Type
Checklist
Additional Information
Branch Naming